Add sanitizer support using GCC toolchain features#64
Add sanitizer support using GCC toolchain features#64RSingh1511 wants to merge 4 commits intoeclipse-score:mainfrom
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
90a54e3 to
698d31d
Compare
MODULE.bazel
Outdated
| bazel_dep(name = "score_bazel_platforms", version = "0.0.4") | ||
| bazel_dep(name = "score_docs_as_code", version = "2.2.0") | ||
| bazel_dep(name = "score_tooling", version = "1.1.1") | ||
| bazel_dep(name = "score_docs_as_code", version = "2.3.0") |
There was a problem hiding this comment.
You will need docs-as-code 3.0.1 if you want to use tooling 1.1.2.
There was a problem hiding this comment.
I've verified that score_docs_as_code@2.3.0 works correctly with score_tooling@1.1.2 and all tests pass. The sanitizer implementation is complete and ready for review.
Using 3.0.1 causes CI failures because score_tooling@1.1.2 declares it as dev_dependency = True, making it invisible to consuming modules.
There was a problem hiding this comment.
Do you need the new added Documentation API this from tooling 1.1.2?
If not then downgrading to 1.1.0 and upgrading docs-as-code to 3.0.0 would be advisable.
Otherwise you will not be able to use the new features from docs-as-code.
See Features Here
There was a problem hiding this comment.
Done! Downgraded to score_tooling@1.1.0.
Keeping score_docs_as_code@2.3.0 due to compatibility with score_baselibs (all released versions require 2.3.0). Upgrading to 3.0.0 causes CI failures.
6500360 to
630e545
Compare
630e545 to
a218e21
Compare
a218e21 to
f19c4ff
Compare
f19c4ff to
008bf6c
Compare
pawelrutkaq
left a comment
There was a problem hiding this comment.
Sorry we need to postone it until logging in in shape and works with other repos. Then I will review and recheck.
| @@ -0,0 +1,63 @@ | |||
| # ******************************************************************************* | |||
There was a problem hiding this comment.
why this is seperate workflow ? isnt it something that shall run in coverage and user shall jsut have ability to enable it ? At the end, its running tests
There was a problem hiding this comment.
It's not a good idea to combine coverage instrumentation with sanitizer instrumentation as they can affect each other in gcc: we may get incorrect coverage data, and maybe even get false positives in sanitizers.
MODULE.bazel
Outdated
| bazel_dep(name = "score_bazel_platforms", version = "0.0.4") | ||
| bazel_dep(name = "score_docs_as_code", version = "2.2.0") | ||
| bazel_dep(name = "score_tooling", version = "1.1.1") | ||
| bazel_dep(name = "score_docs_as_code", version = "2.3.0") |
| coverage --cxxopt -runtime-counter-relocation | ||
|
|
||
| # Dynamic analysis (sanitizers) for Linux host builds/tests. | ||
| try-import %workspace%/quality/sanitizer/sanitizer.bazelrc |
There was a problem hiding this comment.
i would keep it into bazelrc for now.
.github/workflows/sanitizers.yml
Outdated
| set -euo pipefail | ||
| echo "Running: bazel test --config=${{ matrix.sanitizer_config }} //tests/cpp/..." | ||
| # Note: Only testing C++ targets as Rust sanitizers require different configuration | ||
| bazel test --config=${{ matrix.sanitizer_config }} //tests/cpp/... |
There was a problem hiding this comment.
this is not correct target ? we shall remove contant od tests as they ment to be for cit/fit. Run correct target and let see how long it takes
d781c59 to
1ba5d7e
Compare
Rust targets fail with C++ sanitizer flags due to linking incompatibilities
56dce0f to
ce8a6f7
Compare
| test:asan_ubsan_lsan --test_env=ASAN_OPTIONS=exitcode=55:allow_addr2line=1:verbosity=1:detect_leaks=1:halt_on_error=1:allocator_may_return_null=1 | ||
| test:asan_ubsan_lsan --test_env=UBSAN_OPTIONS=exitcode=55:allow_addr2line=1:verbosity=1:print_stacktrace=1:halt_on_error=1 |
There was a problem hiding this comment.
such env settings for sanitizers should instead come from a common place to prevent that every sub-repo configures them differently
There was a problem hiding this comment.
@stmuench Good point! This aligns with @4og suggestion for a
centralized C++ policies repo (similar to https://github.com/eclipse-score/score_rust_policies)
| test:tsan --compilation_mode=dbg | ||
| test:tsan --features=tsan | ||
| test:tsan --platform_suffix=tsan | ||
| test:tsan --test_env=TSAN_OPTIONS=exitcode=55:allow_addr2line=1:verbosity=1:halt_on_error=1:detect_deadlocks=1 |
Notes for Reviewer
Pre-Review Checklist for the PR Author
Checklist for the PR Reviewer
Post-review Checklist for the PR Author
References
Closes #